-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix HTML filter issue when the text contains malformed HTML tags #27
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Some small adjustments tho.
@@ -258,6 +258,10 @@ function ($match) { | |||
*/ | |||
private function isIgnoredTag(?string $name): bool | |||
{ | |||
if ($name === null) { | |||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the tag should be ignored if its malformed. So this should return true
.
Or else you will get a array access on null
in
php-speller/src/Source/Filter/HtmlFilter.php
Line 123 in 629e86d
} elseif ('/' === $tagName[0]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @icanhazstring .
Actually if the tag is ignored, the result of the filter becomes foo/
because the state is not restored and the text after the character >
is replaced by spaces. Do you have any idea to solve it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The base problem here I think is: The script "thinks" it is parsing a closing tag with >
but it shouldn't. Because it never started one using <
.
Maybe there is a total underlying problem here with the parsing method.
So I would go with your initial solution as it is the more correct one.
So your case with $tagName !== null
should be in here.
Maybe we can think of some other solution someday :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing :)
{ | ||
$filter = new HtmlFilter(); | ||
$html = "foo/>bar<br><br/>"; | ||
$text = "foo/ bar "; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the tag will be ignored the outcome should be foo/>bar
Fixes issue #26